Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use UTF-8 to write sitecustomize.py #9136

Closed
wants to merge 2 commits into from
Closed

Conversation

pfmoore
Copy link
Member

@pfmoore pfmoore commented Nov 14, 2020

No description provided.

@pfmoore
Copy link
Member Author

pfmoore commented Nov 14, 2020

Fixes #9135

@pfmoore
Copy link
Member Author

pfmoore commented Nov 14, 2020

Failing at the moment, because Python 2 open() doesn't have an encoding argument 🙁

Let's leave this till we remove Python 2 tests.

@xavfernandez
Copy link
Member

io.open should work in all versions I think

@pfmoore
Copy link
Member Author

pfmoore commented Nov 14, 2020

Yeah, probably. But I don't see the point if we're likely to rip it out in a couple of months. I'm happy if someone else wants to make this Python 2 compatible, though.

@pradyunsg
Copy link
Member

How do you feel about waiting a couple of weeks, and then we'll revisit this, because master won't be Python 2?

@uranusjr
Copy link
Member

uranusjr commented Nov 15, 2020

This is also related to #9054 (which I believe would also be resolved by this fix). io.open() returns unicode on Python 2, but pip uses str all over the place. Switching to it would cause a cascade of unicode coersion issues elsewhere, and likely break things that rely on platform-specific encoding (and are currently working). I agree with Paul this should not be backported to Python 2.

OTOH I really would like this to be fixed for Python 3 before we drop Python 2, otherwise there would be no migration path available for people stuck on Python 2 due to this issue (may be a hypothetical problem at this point though). Maybe we can do something like

if six.PY2:
    f = open(...)
else:
    f = open(..., encoding="utf8")
with f:
    f.write(...)

@pfmoore
Copy link
Member Author

pfmoore commented Nov 15, 2020

@uranusjr I have no objection if you want to make a version of this fix that supports Python 2, but I don't expect to do so myself.

@pfmoore
Copy link
Member Author

pfmoore commented Nov 15, 2020

This is also related to #9054

Ah yes, I knew I remembered this issue having come up recently, but I couldn't find it. Thanks for the link.

@pfmoore pfmoore closed this Jan 24, 2021
@pfmoore pfmoore deleted the issue_9135 branch January 24, 2021 11:13
@domdfcoding
Copy link
Contributor

Was there a reason this was closed? The issue still seems to be present.

@uranusjr
Copy link
Member

uranusjr commented Jun 6, 2021

IIRC this was abandoned because Python 2 was too difficult to handle. Since that is no longer an issue, feel free to submit a new PR.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants